-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SIMSBIOHUB-466: Add Observation Measurement Columns #1213
Conversation
- Manage column components (initial structure) - Autocomplete with stubbed response (issue: input clearing on option select) - Not wired up to sims backend yet - Not wired up to observations table yet
…to SIMSBIOHUB-466
…to SIMSBIOHUB-466
…to SIMSBIOHUB-466
…to SIMSBIOHUB-466
…umns to measurement columns.
…to SIMSBIOHUB-466
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1213 +/- ##
===========================================
+ Coverage 56.26% 76.02% +19.75%
===========================================
Files 576 235 -341
Lines 17647 9629 -8018
Branches 2751 1353 -1398
===========================================
- Hits 9929 7320 -2609
+ Misses 7056 1896 -5160
+ Partials 662 413 -249 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ran into a few issues when testing this locally:
Validation Banner doesn't show
- Hide required columns (like the sample sites)
- Add new row and fill
- Save (see error dialog)
- No banner
Hiding newly added columns
- Add new measurement
- New column cannot be toggled
- Close/ Reopen measurements panel
- Newly added measurement is still cannot be toggled
Deleting Columns from measurement popup
- Removing a single measurement from the list seems to work as expected
- Removing all added measurements seems to reload the panel and add columns back to list (might be data related)
api/src/paths/project/{projectId}/survey/{surveyId}/observations/index.ts
Outdated
Show resolved
Hide resolved
api/src/paths/project/{projectId}/survey/{surveyId}/observations/index.ts
Outdated
Show resolved
Hide resolved
app/src/components/data-grid/taxonomy/TaxonomyDataGridViewCell.tsx
Outdated
Show resolved
Hide resolved
app/src/features/surveys/observations/measurements/list/MeasurementsListCard.tsx
Show resolved
Hide resolved
app/src/features/surveys/observations/measurements/search/MeasurementsSearchAutocomplete.tsx
Outdated
Show resolved
Hide resolved
app/src/features/surveys/observations/measurements/search/MeasurementsSearchAutocomplete.tsx
Show resolved
Hide resolved
...atures/surveys/observations/observations-table/configure-table/ConfigureColumnsContainer.tsx
Outdated
Show resolved
Hide resolved
I think this is an existing issue. It seems that we only validate the sampling columns if at least one of them is populated, probably to do with the observation import maybe? The comments in that code talk about not validating sampling columns if its an incidental observation.
I fixed a few issues with the visibility controls. I think it should be happy now.
Yeah this is a more interesting issue. The problem is that the measurement data is still part of the row data, and just with the way the useEffect is set up right now, when you remove all measurement columns it re-loads the columns from the existing data, hence those columns showing up again suddenly. There should be a fix, but to date we only ever do row based deletes, so ill have to poke at it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retested this and saw something else:
- Add a new row
- Discard Changes
- Open Measurement Panel
- Cannot add measurements (probably an issue with state)
Should be fixed. Was not resetting the row modes model on discard |
app/src/features/surveys/observations/measurements/dialog/MeasurementsDialog.tsx
Outdated
Show resolved
Hide resolved
...atures/surveys/observations/observations-table/configure-table/ConfigureColumnsContainer.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! 📏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍾
63af389
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚒️
Quality Gate failedFailed conditions |
Links to Jira Tickets
SIMSBIOHUB-466
Description of Changes
Many updates to the Observation Table components and related components
When creating an observation record, a new subcount record is also created.
Addressed a handful of dependency array warnings and "update a component while rendering another" warnings
Some misc package updates + related fixes.
Pending Critterbase updates
Wire up the stubbed calls to Critterbase, once its new endpoints are ready.
TODOs for another ticket
Update the telemetry datagrid components/tables to use the latest updates from the survey observations components.
Fix the 3-4 app tests that started to fail (probably a side effect of package updates?) I tried fixed them but was making no progress, so they are skipped for now. They are all related to trying to capture select box clicks, which is historically tricky.
Testing Notes
Observations table works as expected, when adding and removing observations with and without measurement columns.